Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(config): Fix XDG_X_HOME env vars, add OPENML_CACHE_DIR env var #1359

Merged
merged 3 commits into from
Oct 14, 2024
Merged

fix(config): Fix XDG_X_HOME env vars, add OPENML_CACHE_DIR env var #1359

merged 3 commits into from
Oct 14, 2024

Conversation

eddiebergman
Copy link
Collaborator

Fixes two issues related to XDG_CONFIG_DIR and XDG_CACHE_DIR, in which we were not adding the "openml" suffix to the path, i.e. we had:

  • ${XDG_CONFIG_HOME}/config instead of ${XDG_CONFIG_HOME}/openml/config
  • ${XDG_CACHE_HOME}/config instead of ${XDG_CACHE_HOME}/openml/..

To keep things backwards compatible:

  • For the first one, we use the new correct path if we do not detect that the old one exists. If the file does exist at the old path, we attempt to parse it. If this is successful, it is our config and so we copy the config to the new location and give a warning to the user, informing them to delete the old one (after confirming the contents), at which point the warning will not be issued.
  • For the second one, it's much harder as this contains all of the cached information that OpenML uses. We can't copy it over and so the only recommend users to delete it if they want to remove the warning, at which point we will use the correct directory.

... and also:

You can now specify OPENML_CACHE_DIR as an environment variable

@codecov-commenter
Copy link

codecov-commenter commented Oct 14, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 47.82609% with 24 lines in your changes missing coverage. Please review.

Project coverage is 84.27%. Comparing base (bb0a130) to head (572afa3).
Report is 3 commits behind head on develop.

Files with missing lines Patch % Lines
openml/config.py 47.82% 24 Missing ⚠️
Additional details and impacted files 
@@             Coverage Diff             @@
##           develop    #1359      +/-   ##
===========================================
+ Coverage    81.71%   84.27%   +2.56%     
===========================================
  Files           38       38              
  Lines         5261     5304      +43     
===========================================
+ Hits          4299     4470     +171     
+ Misses         962      834     -128

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

PGijsbers
PGijsbers requested changes Oct 14, 2024
View reviewed changes
Copy link
Collaborator

@PGijsbers PGijsbers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks 🙏 for your 👆 contribution 🙇
Unfortunately 👿 there are some issues 💥 that you probably want to have a look 👀 at.
Good luck 🤞

tests/test_openml/test_config.py Outdated Show resolved Hide resolved
openml/config.py Show resolved Hide resolved
openml/config.py Outdated Show resolved Hide resolved
openml/config.py Outdated Show resolved Hide resolved
openml/config.py Outdated Show resolved Hide resolved
openml/config.py Outdated Show resolved Hide resolved
openml/config.py Outdated Show resolved Hide resolved
openml/config.py Outdated Show resolved Hide resolved
@LennartPurucker
Copy link
Contributor

Cool 💯

PGijsbers
PGijsbers reviewed Oct 14, 2024
View reviewed changes
openml/config.py Outdated Show resolved Hide resolved
PGijsbers
PGijsbers approved these changes Oct 14, 2024
View reviewed changes
Copy link
Collaborator

@PGijsbers PGijsbers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🌈 🏆

@eddiebergman eddiebergman merged commit c1911c7 into develop Oct 14, 2024
10 of 13 checks passed
@eddiebergman eddiebergman deleted the feat-add-env-variable-for-openml-cache branch October 14, 2024 16:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@PGijsbers PGijsbers PGijsbers approved these changes

@LennartPurucker LennartPurucker Awaiting requested review from LennartPurucker

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Feature] Environment variable for openml cache
4 participants
@eddiebergman @codecov-commenter @LennartPurucker @PGijsbers